Compose tab for MUSH poses, and post-crash diagnostics log viewer - #27
Merged
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015MPRzndB5egy4F2A3q5852
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015MPRzndB5egy4F2A3q5852
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015MPRzndB5egy4F2A3q5852
…entDefault flag Ctrl/Cmd+Enter never suppressed the textarea's native newline insertion, so the default DOM mutation and its paired oninput could race the async SendAsync() and leave a stray newline in the draft after sending. @onkeydown:preventDefault can only react to the *previous* render, so arm it from the modifier's own keydown, one event ahead, while the actual send decision keeps using the Enter event's own accurate CtrlKey/MetaKey. Add coverage for both halves of the chord. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015MPRzndB5egy4F2A3q5852
…rd test coverage The guard suppressed the native default for whatever key followed a Ctrl/Cmd keydown, breaking Ctrl+C/V/A/Z inside the textarea for that one keystroke — a worse regression than the race it guarded against. The race itself doesn't survive scrutiny: MushPoseFormatter drops trailing blank lines, so a stray newline can never reach the wire, and either event ordering converges on an empty textarea since Blazor diffs value="@Vm.Body" against its own last-rendered value, not the live DOM. OnKeyDown reverts to the plain Ctrl/Meta+Enter check; the two keyboard tests from the previous round stay. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015MPRzndB5egy4F2A3q5852
…Core Also bumps SharpClient.UI's Microsoft.Extensions.DependencyInjection.Abstractions pin to 10.0.9 to match the version Core now pulls in transitively via Microsoft.Extensions.Logging.Abstractions, resolving an NU1605 downgrade error in the Android head build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015MPRzndB5egy4F2A3q5852
…Unit tests bunit 2.7.2 resolves AngleSharp 1.4.0 transitively. A NuGet advisory for that version now fails the audit, and TreatWarningsAsErrors turns NU1902 into a build error, so the UI test project stopped building on every branch including master. Pinning the patched version restores the build without weakening the audit. Matches the pin SharpMUSH applied for the same advisory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015MPRzndB5egy4F2A3q5852
… gating - Compose: keep a fallback draft while no session is active and adopt it into the first session that becomes active, instead of discarding text typed before connecting. - Diagnostics: re-apply InitialFilter on OnParametersSet (not just on init) so the crash banner's deep link works while already on /diagnostics, without clobbering a manually chosen filter chip. - Settings: gate the "View log" row on ILogReader.IsAvailable and "Export log" on ILogExporter.IsAvailable, matching the design doc, instead of gating both off the exporter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015MPRzndB5egy4F2A3q5852
- Clear() now empties the pending draft when no session is active, so the button isn't a dead no-op now that Body persists text typed before a session connects. - Add SettingsView coverage proving the View log / Export log rows are gated independently (reader-only and exporter-only host combinations). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015MPRzndB5egy4F2A3q5852
WalkthroughAdds a Compose workflow with MUSH formatting, session drafts, UI controls, and navigation. Moves diagnostics into Core with file logging, crash markers, readers, viewing, filtering, host registration, and tests. ChangesCompose and diagnostics
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)Compose send flowsequenceDiagram
participant ComposeView
participant ComposeViewModel
participant MushPoseFormatter
participant ISession
ComposeView->>ComposeViewModel: Submit body and prefix
ComposeViewModel->>MushPoseFormatter: Format command
MushPoseFormatter-->>ComposeViewModel: Return formatted command
ComposeViewModel->>ISession: Send formatted line
ISession-->>ComposeViewModel: Complete send
ComposeViewModel-->>ComposeView: Clear draft and refresh
Crash diagnostics flowsequenceDiagram
participant CrashBanner
participant ILogReader
participant DiagnosticsPage
participant DiagnosticsView
CrashBanner->>ILogReader: Load pending crash
ILogReader-->>CrashBanner: Return CrashReport
CrashBanner->>DiagnosticsPage: Navigate with crash filter
DiagnosticsPage->>DiagnosticsView: Pass filter parameter
DiagnosticsView->>ILogReader: Read diagnostic entries
ILogReader-->>DiagnosticsView: Return log entries
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
There was a problem hiding this comment.
Pull request overview
Adds two end-user features to SharpClient’s UI and Core layers: (1) a dedicated Compose tab for writing multi-line MUSH poses with deterministic wire-format escaping, and (2) an in-app diagnostics log viewer with a “crashed last run” banner backed by a Core-level file log reader/parser.
Changes:
- Introduces
/compose(nav tab) withMushPoseFormatter+ComposeViewModeland a full-height editor/preview UI. - Moves diagnostics logging primitives into
SharpClient.Core, adds parsing + reading + crash-marker support, and exposes them via/diagnosticsplus aCrashBanner. - Expands test coverage (TUnit + bUnit) for compose + diagnostics flows and pins AngleSharp in UI tests to satisfy NuGet audit.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/SharpClient.UI.Tests/UiFakeLogReader.cs | Adds a UI-test ILogReader fake for diagnostics and banner tests. |
| tests/SharpClient.UI.Tests/SharpClient.UI.Tests.csproj | Pins AngleSharp to a patched version for audit compliance. |
| tests/SharpClient.UI.Tests/SettingsViewTests.cs | Updates Settings tests for new diagnostics reader/exporter gating and UI rows. |
| tests/SharpClient.UI.Tests/DiagnosticsViewTests.cs | Adds bUnit coverage for diagnostics filtering/actions/empty state. |
| tests/SharpClient.UI.Tests/CrashBannerTests.cs | Adds bUnit coverage for crash banner render + dismiss behavior. |
| tests/SharpClient.UI.Tests/ComposeViewTests.cs | Adds bUnit coverage for compose UI interactions and send behavior. |
| tests/SharpClient.Tests/Presentation/ComposeViewModelTests.cs | Adds TUnit coverage for compose VM send gating, drafts, persistence. |
| tests/SharpClient.Tests/Formatting/MushPoseFormatterTests.cs | Adds TUnit coverage for MUSH escaping/joining contract. |
| tests/SharpClient.Tests/Diagnostics/LogEntryParserTests.cs | Adds TUnit coverage for parsing the log format and edge cases. |
| tests/SharpClient.Tests/Diagnostics/FileLogStoreTests.cs | Adds TUnit coverage for file log writing + rotation behavior. |
| tests/SharpClient.Tests/Diagnostics/FileLogReaderTests.cs | Adds TUnit coverage for reader ordering, truncation, crash marker lifecycle. |
| src/SharpClient.Web/Program.cs | Registers NoopLogReader for the web host. |
| src/SharpClient.UI/wwwroot/sc-interop.js | Adds clipboard helper copyText for diagnostics Copy action. |
| src/SharpClient.UI/wwwroot/app.css | Adds styling for compose page, diagnostics page, and crash banner. |
| src/SharpClient.UI/SharpClient.UI.csproj | Updates DI abstractions package version. |
| src/SharpClient.UI/ServiceCollectionExtensions.cs | Registers ComposeViewModel for both hosts. |
| src/SharpClient.UI/Pages/DiagnosticsPage.razor | Adds /diagnostics route with query-param filter wiring. |
| src/SharpClient.UI/Pages/ComposePage.razor | Adds /compose route and injects the compose VM. |
| src/SharpClient.UI/Layout/MainLayout.razor | Adds CrashBanner and adds Compose nav entry. |
| src/SharpClient.UI/Components/SettingsView.razor | Adds “View log” row and shows Diagnostics section if reader/exporter available. |
| src/SharpClient.UI/Components/DiagnosticsView.razor | Implements diagnostics viewer UI, filters, and actions. |
| src/SharpClient.UI/Components/CrashBanner.razor | Implements “crashed last run” banner with deep-link + dismiss. |
| src/SharpClient.UI/Components/ComposeView.razor | Implements compose editor/preview UI with prefix chips and send chord. |
| src/SharpClient.Core/SharpClient.Core.csproj | Adds logging abstractions dependency needed by Core diagnostics logger provider. |
| src/SharpClient.Core/Presentation/ComposeViewModel.cs | Implements per-session compose drafts + prefix persistence + send logic. |
| src/SharpClient.Core/Formatting/MushPoseFormatter.cs | Implements the wire-format escaping and prefix/body joining rules. |
| src/SharpClient.Core/Diagnostics/LogEntryParser.cs | Parses the file log format into LogEntry objects with multi-line detail. |
| src/SharpClient.Core/Diagnostics/LogEntry.cs | Introduces LogEntry and CrashReport record types. |
| src/SharpClient.Core/Diagnostics/ILogReader.cs | Adds ILogReader abstraction + NoopLogReader. |
| src/SharpClient.Core/Diagnostics/FileLogStore.cs | Makes the log store platform-agnostic and adds crash marker sidecar. |
| src/SharpClient.Core/Diagnostics/FileLogReader.cs | Implements merged rotated-log reading and crash marker reading/clearing. |
| src/SharpClient.Core/Diagnostics/FileLoggerProvider.cs | Moves logger provider into Core diagnostics namespace. |
| src/SharpClient.App/Platforms/Android/MainActivity.cs | Updates namespace imports for moved diagnostics types. |
| src/SharpClient.App/MauiProgram.cs | Supplies log directory to Core log store and registers ILogReader. |
| docs/superpowers/specs/2026-08-10-compose-and-diagnostics-design.md | Adds design spec for compose + diagnostics features. |
| docs/superpowers/plans/2026-08-10-compose-and-diagnostics.md | Adds detailed implementation plan and constraints. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+133
to
+135
| var text = string.Join('\n', Visible.Select(e => | ||
| $"{e.Timestamp:yyyy-MM-dd HH:mm:ss} [{e.Level}] {e.Category}: {e.Message}" | ||
| + (e.Detail is null ? string.Empty : "\n" + e.Detail))); |
Comment on lines
+11
to
+22
| public Task<IReadOnlyList<LogEntry>> ReadAsync(int maxEntries = 500) | ||
| { | ||
| var entries = new List<LogEntry>(); | ||
| entries.AddRange(ReadFile(_store.BackupPath)); | ||
| entries.AddRange(ReadFile(_store.FilePath)); | ||
|
|
||
| var start = Math.Max(0, entries.Count - maxEntries); | ||
| var newest = entries.GetRange(start, entries.Count - start); | ||
| newest.Reverse(); | ||
|
|
||
| return Task.FromResult<IReadOnlyList<LogEntry>>(newest); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-08-10-compose-and-diagnostics.md`:
- Around line 2354-2358: Update CopyAsync so each copied entry omits the “: ”
separator when e.Category is empty, while retaining the existing “{Category}:
{Message}” format for categorized entries and preserving the timestamp, level,
and detail formatting.
- Around line 1835-1841: Update WriteException to always record the exception
but only call WriteCrashMarker for process-level unhandled-exception sources;
ensure the TaskScheduler.UnobservedTaskException path in MauiProgram remains
log-only after e.SetObserved(), preventing recoverable task faults from creating
last-crash.txt or a crash banner.
- Around line 1847-1857: Update the write flow around Write, WriteException, and
WriteCrashMarker so the log write and crash-marker write execute under the same
_gate lock. Keep the lock held through the complete combined operation, rather
than releasing it before WriteCrashMarker, while preserving the existing
failure-swallowing behavior for marker writes.
- Around line 2640-2647: Update the MAUI crash validation steps after the Web
host checks to trigger a process-level failure reaching
AppDomain.UnhandledException or AndroidEnvironment.UnhandledExceptionRaiser,
rather than throwing from a Blazor component and hard-killing the app.
Alternatively, direct the test through an existing seam that writes the crash
marker, then relaunch the app and verify CrashBanner and the Crashes-filter
entry.
- Around line 2402-2406: Update DiagnosticsPage to check ILogReader.IsAvailable
before rendering DiagnosticsView, hiding the diagnostics route or redirecting
when the Web host uses NoopLogReader. Preserve the existing filter behavior for
available readers, and add a direct-navigation test verifying the Web host does
not expose the diagnostics viewer or its actions.
- Around line 1921-1931: Update ReadAsync to handle maxEntries values less than
or equal to zero before calculating the range, returning an empty IReadOnlyList
for those inputs; preserve the existing newest-entry ordering and limiting
behavior for positive values.
- Around line 630-642: Update ComposeView’s Body getter and setter to preserve
text entered while Active is null by reusing the existing _pendingDraft pattern
from ComposeViewModel. Return the pending value when no session is active, store
edits there instead of discarding them, and transfer it into the active draft
when a session becomes active. Add a regression test covering entry before
activation and preservation after activation.
- Around line 1606-1607: Update the timestamp parsing in FileLogReader.ReadFile
to use TryParseExact instead of ParseExact, and skip the current header line
when the timestamp is invalid. Preserve parsing of valid entries so one
malformed timestamp does not discard the entire file.
- Around line 1014-1020: Update ComposeView’s OnKeyDown handler to conditionally
prevent the browser default action when Enter is pressed with CtrlKey or
MetaKey, while leaving bare Enter unchanged for multiline input. Apply the
equivalent client-side prevention in both the interactive Web host and MAUI
Blazor WebView host implementations, alongside the existing SendAsync call.
In `@src/SharpClient.Core/Diagnostics/FileLogReader.cs`:
- Around line 11-21: Update the public ReadAsync method to validate maxEntries
before calculating start or calling GetRange, rejecting negative values with the
established argument-validation behavior; preserve the existing ordering and
limiting behavior for zero and positive values.
In `@src/SharpClient.Core/Diagnostics/FileLogStore.cs`:
- Around line 44-50: Update FileLogStore.WriteException so it records the
exception without creating last-crash.txt by default, and add a separate
termination-aware path or parameter that creates the marker only for handlers
indicating process termination. Change the TaskScheduler.UnobservedTaskException
handling in MauiProgram to use the non-terminating append path before
SetObserved, while preserving crash-marker creation for terminating handlers.
Add a regression test covering this lifecycle and verifying no false crash
banner marker is left.
In `@src/SharpClient.UI/Components/ComposeView.razor`:
- Around line 85-91: Update the ComposeView key handling around OnKeyDown so
Ctrl/Cmd+Enter is processed by a DOM listener that calls preventDefault() only
for that shortcut, preventing the textarea from inserting a newline while
preserving bare Enter behavior. Retain SendAsync() for the shortcut and add a
browser-level test verifying the editor remains empty after sending.
In `@src/SharpClient.UI/Pages/DiagnosticsPage.razor`:
- Around line 1-5: Update the DiagnosticsPage component to inject ILogReader as
Reader and conditionally render DiagnosticsView only when Reader.IsAvailable is
true, while preserving the existing InitialFilter binding.
In `@tests/SharpClient.UI.Tests/UiFakeLogReader.cs`:
- Around line 14-15: Update UiFakeLogReader.ReadAsync to return a snapshot of
Entries limited to maxEntries rather than the entire collection. Preserve the
IReadOnlyList<LogEntry> result and ensure the original Entries collection is not
modified.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ccd1bf9-b6d9-4b00-956b-a358aef5a27c
📒 Files selected for processing (36)
docs/superpowers/plans/2026-08-10-compose-and-diagnostics.mddocs/superpowers/specs/2026-08-10-compose-and-diagnostics-design.mdsrc/SharpClient.App/MauiProgram.cssrc/SharpClient.App/Platforms/Android/MainActivity.cssrc/SharpClient.Core/Diagnostics/FileLogReader.cssrc/SharpClient.Core/Diagnostics/FileLogStore.cssrc/SharpClient.Core/Diagnostics/FileLoggerProvider.cssrc/SharpClient.Core/Diagnostics/ILogReader.cssrc/SharpClient.Core/Diagnostics/LogEntry.cssrc/SharpClient.Core/Diagnostics/LogEntryParser.cssrc/SharpClient.Core/Formatting/MushPoseFormatter.cssrc/SharpClient.Core/Presentation/ComposeViewModel.cssrc/SharpClient.Core/SharpClient.Core.csprojsrc/SharpClient.UI/Components/ComposeView.razorsrc/SharpClient.UI/Components/CrashBanner.razorsrc/SharpClient.UI/Components/DiagnosticsView.razorsrc/SharpClient.UI/Components/SettingsView.razorsrc/SharpClient.UI/Layout/MainLayout.razorsrc/SharpClient.UI/Pages/ComposePage.razorsrc/SharpClient.UI/Pages/DiagnosticsPage.razorsrc/SharpClient.UI/ServiceCollectionExtensions.cssrc/SharpClient.UI/SharpClient.UI.csprojsrc/SharpClient.UI/wwwroot/app.csssrc/SharpClient.UI/wwwroot/sc-interop.jssrc/SharpClient.Web/Program.cstests/SharpClient.Tests/Diagnostics/FileLogReaderTests.cstests/SharpClient.Tests/Diagnostics/FileLogStoreTests.cstests/SharpClient.Tests/Diagnostics/LogEntryParserTests.cstests/SharpClient.Tests/Formatting/MushPoseFormatterTests.cstests/SharpClient.Tests/Presentation/ComposeViewModelTests.cstests/SharpClient.UI.Tests/ComposeViewTests.cstests/SharpClient.UI.Tests/CrashBannerTests.cstests/SharpClient.UI.Tests/DiagnosticsViewTests.cstests/SharpClient.UI.Tests/SettingsViewTests.cstests/SharpClient.UI.Tests/SharpClient.UI.Tests.csprojtests/SharpClient.UI.Tests/UiFakeLogReader.cs
Comment on lines
+630
to
+642
| public string Body | ||
| { | ||
| get => Active is not null && _drafts.TryGetValue(Active, out var draft) ? draft : string.Empty; | ||
| set | ||
| { | ||
| if (Active is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _drafts[Active] = value; | ||
| Changed?.Invoke(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve drafts entered before session activation.
ComposeView remains editable when Active is null, but this setter discards every value and the getter returns an empty string. Users can type a draft, connect a session, and lose the draft.
Store a pending draft, or disable the editor when no session is active. Add a regression test. The downstream src/SharpClient.Core/Presentation/ComposeViewModel.cs implementation already uses _pendingDraft.
Proposed fix
+ private string _pendingDraft = string.Empty;
+
public string Body
{
- get => Active is not null && _drafts.TryGetValue(Active, out var draft) ? draft : string.Empty;
+ get => Active is null
+ ? _pendingDraft
+ : _drafts.TryGetValue(Active, out var draft) ? draft : string.Empty;
set
{
if (Active is null)
{
+ _pendingDraft = value;
+ Changed?.Invoke();
return;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-10-compose-and-diagnostics.md` around lines
630 - 642, Update ComposeView’s Body getter and setter to preserve text entered
while Active is null by reusing the existing _pendingDraft pattern from
ComposeViewModel. Return the pending value when no session is active, store
edits there instead of discarding them, and transfer it into the active draft
when a session becomes active. Add a regression test covering entry before
activation and preservation after activation.
Comment on lines
+1014
to
+1020
| // Ctrl/Cmd+Enter sends; a bare Enter has to stay a newline in a multi-line composer. | ||
| private async Task OnKeyDown(KeyboardEventArgs e) | ||
| { | ||
| if (e.Key == "Enter" && (e.CtrlKey || e.MetaKey)) | ||
| { | ||
| await SendAsync(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="docs/superpowers/plans/2026-08-10-compose-and-diagnostics.md"
printf '%s\n' '--- target section ---'
sed -n '980,1045p' "$file"
printf '%s\n' '--- relevant symbols and prevention APIs ---'
rg -n -C 3 'OnKeyDown|preventDefault|KeyboardEventArgs|SendAsync|ComposeView|textarea' "$file"Repository: SharpMUSH/SharpClient
Length of output: 15186
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- project targets and existing event modifiers ---'
rg -n -C 3 'TargetFramework|Microsoft\.AspNetCore\.Components|`@onkeydown`|preventDefault|KeyboardEventArgs' \
--glob '*.csproj' --glob '*.razor' --glob '*.cs' --glob '*.md' .
printf '%s\n' '--- compose test plan around keyboard behavior ---'
sed -n '768,935p' docs/superpowers/plans/2026-08-10-compose-and-diagnostics.mdRepository: SharpMUSH/SharpClient
Length of output: 39586
🌐 Web query:
ASP.NET Core Blazor event handling preventDefault keyboard event textarea @onkeydown:preventDefault official documentation
💡 Result:
In ASP.NET Core Blazor, you can prevent the default browser behavior for keyboard events—such as typing characters into a textarea—by using the @on{DOM EVENT}:preventDefault directive attribute [1][2]. To prevent a keydown event's default action (e.g., stopping a character from appearing in a textarea), add @onkeydown:preventDefault to the element [1][3]. Usage Examples: 1. Static Prevention: To unconditionally prevent the default behavior, include the attribute without a value (which is equivalent to setting it to true) [1][2]: <textarea @onkeydown:preventDefault></textarea> 2. Conditional Prevention: You can also bind the attribute to a boolean expression to dynamically decide whether to prevent the default action [1][2]: <textarea @onkeydown:preventDefault="shouldPreventDefault" /> @code { private bool shouldPreventDefault = true; } Key Considerations: - Synchronous Execution: Because Blazor event handling (especially in Blazor Server) is asynchronous, you cannot call preventDefault inside a C# event handler method after the event has already been processed [4][5]. The @on{DOM EVENT}:preventDefault directive works because it allows Blazor to instruct the browser to prevent the default action before the event is sent to your C# code [6]. - Combining Attributes: You can use @onkeydown:preventDefault alongside a standard event handler [1][3]: <textarea @onkeydown="HandleKeyDown" @onkeydown:preventDefault />
Citations:
- 1: https://learn.microsoft.com/en-us/aspnet/core/blazor/components/event-handling?view=aspnetcore-10.0
- 2: https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/blazor/components/event-handling.md
- 3: https://learn.microsoft.com/en-us/aspnet/core/blazor/components/event-handling?view=aspnetcore-9.0
- 4: How to preventDefault when handle event like js dotnet/aspnetcore#33422
- 5: https://blazor-university.com/components/component-events/browser-dom-events
- 6: https://www.meziantou.net/how-to-use-event-preventdefault-in-blazor.htm
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- implemented ComposeView and keyboard tests ---'
sed -n '1,125p' src/SharpClient.UI/Components/ComposeView.razor
sed -n '1,120p' tests/SharpClient.UI.Tests/ComposeViewTests.cs
printf '%s\n' '--- host render modes and WebView setup ---'
rg -n -C 4 'AddInteractive|RenderMode|BlazorWebView|RootComponents|SharpClient.UI|MapRazorComponents|UseStaticFiles' \
src --glob '*.cs' --glob '*.razor' --glob '*.csproj'Repository: SharpMUSH/SharpClient
Length of output: 21113
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
component = Path("src/SharpClient.UI/Components/ComposeView.razor").read_text()
web_program = Path("src/SharpClient.Web/Program.cs").read_text()
maui_program = Path("src/SharpClient.App/MauiProgram.cs").read_text()
textarea = re.search(r"<textarea\b(?P<body>.*?)</textarea>", component, re.S)
handler = re.search(r"private async Task OnKeyDown\(KeyboardEventArgs e\)(?P<body>.*?)(?=\n private async Task SendAsync)", component, re.S)
assert textarea and handler
print("textarea_has_prevent_default:", ":preventDefault" in textarea.group("body"))
print("handler_checks_modifier_enter:",
'e.Key == "Enter" && (e.CtrlKey || e.MetaKey)' in handler.group("body"))
print("handler_calls_send:", "await SendAsync();" in handler.group("body"))
print("web_uses_interactive_server:", "AddInteractiveServerComponents()" in web_program)
print("maui_uses_blazor_webview:", "AddMauiBlazorWebView()" in maui_program)
PY
printf '%s\n' '--- shared JS interop surface ---'
sed -n '1,220p' src/SharpClient.UI/wwwroot/sc-interop.jsRepository: SharpMUSH/SharpClient
Length of output: 9783
Prevent the default action for modifier+Enter.
ComposeView can insert a newline while SendAsync clears Vm.Body. Add client-side conditional prevention for Enter with CtrlKey or MetaKey. Keep bare Enter available for multiline input. Cover both the interactive Web host and the MAUI Blazor WebView host.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-10-compose-and-diagnostics.md` around lines
1014 - 1020, Update ComposeView’s OnKeyDown handler to conditionally prevent the
browser default action when Enter is pressed with CtrlKey or MetaKey, while
leaving bare Enter unchanged for multiline input. Apply the equivalent
client-side prevention in both the interactive Web host and MAUI Blazor WebView
host implementations, alongside the existing SendAsync call.
Comment on lines
+1606
to
+1607
| timestamp = DateTimeOffset.ParseExact( | ||
| header.Groups["ts"].Value, TimestampFormat, CultureInfo.InvariantCulture); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Skip malformed timestamps instead of failing the complete parse.
ParseExact throws when a header-shaped line contains an invalid date. FileLogReader.ReadFile then catches the exception and returns no entries, so one corrupt line hides all valid entries in that file.
Use TryParseExact and skip invalid headers. This matches the malformed-input requirement and the current parser implementation.
Proposed fix
- timestamp = DateTimeOffset.ParseExact(
- header.Groups["ts"].Value, TimestampFormat, CultureInfo.InvariantCulture);
+ if (!DateTimeOffset.TryParseExact(
+ header.Groups["ts"].Value,
+ TimestampFormat,
+ CultureInfo.InvariantCulture,
+ DateTimeStyles.None,
+ out timestamp))
+ {
+ continue;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| timestamp = DateTimeOffset.ParseExact( | |
| header.Groups["ts"].Value, TimestampFormat, CultureInfo.InvariantCulture); | |
| if (!DateTimeOffset.TryParseExact( | |
| header.Groups["ts"].Value, | |
| TimestampFormat, | |
| CultureInfo.InvariantCulture, | |
| DateTimeStyles.None, | |
| out timestamp)) | |
| { | |
| continue; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-10-compose-and-diagnostics.md` around lines
1606 - 1607, Update the timestamp parsing in FileLogReader.ReadFile to use
TryParseExact instead of ParseExact, and skip the current header line when the
timestamp is invalid. Preserve parsing of valid entries so one malformed
timestamp does not discard the entire file.
Comment on lines
+1835
to
+1841
| /// <summary>Records an unhandled exception captured by one of the global hooks.</summary> | ||
| public void WriteException(string source, Exception? ex) | ||
| { | ||
| var block = FormatEntry("CRASH", source, ex?.Message ?? "(no exception object)", ex); | ||
| Write(block); | ||
| WriteCrashMarker(block); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Separate recoverable task faults from crash markers.
WriteException always writes [CRASH] and last-crash.txt. src/SharpClient.App/MauiProgram.cs calls it from TaskScheduler.UnobservedTaskException and then calls e.SetObserved(). That path does not necessarily terminate the app.
A later launch can therefore show a false crash banner. Keep the exception in the log, but write the crash marker only for process-level unhandled-exception paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-10-compose-and-diagnostics.md` around lines
1835 - 1841, Update WriteException to always record the exception but only call
WriteCrashMarker for process-level unhandled-exception sources; ensure the
TaskScheduler.UnobservedTaskException path in MauiProgram remains log-only after
e.SetObserved(), preventing recoverable task faults from creating last-crash.txt
or a crash banner.
Comment on lines
+1847
to
+1857
| private void WriteCrashMarker(string block) | ||
| { | ||
| try | ||
| { | ||
| File.WriteAllText(CrashMarkerPath, block); | ||
| } | ||
| catch | ||
| { | ||
| // Same contract as the log write: recording a crash must not cause another one. | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Serialize crash-marker writes with log writes.
WriteCrashMarker runs after Write releases _gate. Concurrent WriteException calls can overwrite each other or fail one marker write. The catch block hides that loss.
Protect the complete log-and-marker operation with the same lock.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-10-compose-and-diagnostics.md` around lines
1847 - 1857, Update the write flow around Write, WriteException, and
WriteCrashMarker so the log write and crash-marker write execute under the same
_gate lock. Keep the lock held through the complete combined operation, rather
than releasing it before WriteCrashMarker, while preserving the existing
failure-swallowing behavior for marker writes.
Comment on lines
+11
to
+21
| public Task<IReadOnlyList<LogEntry>> ReadAsync(int maxEntries = 500) | ||
| { | ||
| var entries = new List<LogEntry>(); | ||
| entries.AddRange(ReadFile(_store.BackupPath)); | ||
| entries.AddRange(ReadFile(_store.FilePath)); | ||
|
|
||
| var start = Math.Max(0, entries.Count - maxEntries); | ||
| var newest = entries.GetRange(start, entries.Count - start); | ||
| newest.Reverse(); | ||
|
|
||
| return Task.FromResult<IReadOnlyList<LogEntry>>(newest); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate maxEntries before calculating the range.
If maxEntries is negative, line 18 can pass an invalid count to List.GetRange and throw. Reject negative values at the public API boundary.
Proposed fix
public Task<IReadOnlyList<LogEntry>> ReadAsync(int maxEntries = 500)
{
+ ArgumentOutOfRangeException.ThrowIfNegative(maxEntries);
+
var entries = new List<LogEntry>();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public Task<IReadOnlyList<LogEntry>> ReadAsync(int maxEntries = 500) | |
| { | |
| var entries = new List<LogEntry>(); | |
| entries.AddRange(ReadFile(_store.BackupPath)); | |
| entries.AddRange(ReadFile(_store.FilePath)); | |
| var start = Math.Max(0, entries.Count - maxEntries); | |
| var newest = entries.GetRange(start, entries.Count - start); | |
| newest.Reverse(); | |
| return Task.FromResult<IReadOnlyList<LogEntry>>(newest); | |
| public Task<IReadOnlyList<LogEntry>> ReadAsync(int maxEntries = 500) | |
| { | |
| ArgumentOutOfRangeException.ThrowIfNegative(maxEntries); | |
| var entries = new List<LogEntry>(); | |
| entries.AddRange(ReadFile(_store.BackupPath)); | |
| entries.AddRange(ReadFile(_store.FilePath)); | |
| var start = Math.Max(0, entries.Count - maxEntries); | |
| var newest = entries.GetRange(start, entries.Count - start); | |
| newest.Reverse(); | |
| return Task.FromResult<IReadOnlyList<LogEntry>>(newest); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/SharpClient.Core/Diagnostics/FileLogReader.cs` around lines 11 - 21,
Update the public ReadAsync method to validate maxEntries before calculating
start or calling GetRange, rejecting negative values with the established
argument-validation behavior; preserve the existing ordering and limiting
behavior for zero and positive values.
Comment on lines
+44
to
+50
| /// <summary>Records an unhandled exception captured by one of the global hooks.</summary> | ||
| public void WriteException(string source, Exception? ex) | ||
| { | ||
| var block = FormatEntry("CRASH", source, ex?.Message ?? "(no exception object)", ex); | ||
| Write(block); | ||
| WriteCrashMarker(block); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not create a crash marker for every unhandled task exception.
WriteException always creates last-crash.txt. src/SharpClient.App/MauiProgram.cs invokes this method from TaskScheduler.UnobservedTaskException and then calls SetObserved at lines 62-65. The app can continue and exit cleanly, but the next launch will show a false crash banner.
Use Append for non-terminating task exceptions. Create the crash marker only from handlers that indicate process termination. Add a regression test for this lifecycle.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/SharpClient.Core/Diagnostics/FileLogStore.cs` around lines 44 - 50,
Update FileLogStore.WriteException so it records the exception without creating
last-crash.txt by default, and add a separate termination-aware path or
parameter that creates the marker only for handlers indicating process
termination. Change the TaskScheduler.UnobservedTaskException handling in
MauiProgram to use the non-terminating append path before SetObserved, while
preserving crash-marker creation for terminating handlers. Add a regression test
covering this lifecycle and verifying no false crash banner marker is left.
Comment on lines
+85
to
+91
| // Ctrl/Cmd+Enter sends; a bare Enter has to stay a newline in a multi-line composer. | ||
| private async Task OnKeyDown(KeyboardEventArgs e) | ||
| { | ||
| if (e.Key == "Enter" && (e.CtrlKey || e.MetaKey)) | ||
| { | ||
| await SendAsync(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent the textarea default action for Ctrl/Cmd+Enter.
Line 88 detects the shortcut, but it does not cancel the browser default action. The textarea can insert a newline after SendAsync clears the draft. The following oninput event can then write that newline back to Vm.Body.
Handle this shortcut in a DOM listener. Call preventDefault() only when Key is Enter and Ctrl or Meta is pressed. Keep bare Enter unchanged. Add a browser-level test that verifies the editor remains empty after sending.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/SharpClient.UI/Components/ComposeView.razor` around lines 85 - 91, Update
the ComposeView key handling around OnKeyDown so Ctrl/Cmd+Enter is processed by
a DOM listener that calls preventDefault() only for that shortcut, preventing
the textarea from inserting a newline while preserving bare Enter behavior.
Retain SendAsync() for the shortcut and add a browser-level test verifying the
editor remains empty after sending.
Comment on lines
+1
to
+5
| @page "/diagnostics" | ||
|
|
||
| <PageTitle>SharpClient · Diagnostics</PageTitle> | ||
|
|
||
| <DiagnosticsView InitialFilter="@Filter" /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Hide the diagnostics view when log reading is unavailable.
Line 5 renders DiagnosticsView for every host. A direct navigation to /diagnostics still shows the viewer on the Web host, although the PR objective requires that host not to show it. Inject ILogReader and render the view only when Reader.IsAvailable is true.
Proposed fix
`@page` "/diagnostics"
+@inject ILogReader Reader
<PageTitle>SharpClient · Diagnostics</PageTitle>
-<DiagnosticsView InitialFilter="`@Filter`" />
+@if (Reader.IsAvailable)
+{
+ <DiagnosticsView InitialFilter="`@Filter`" />
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @page "/diagnostics" | |
| <PageTitle>SharpClient · Diagnostics</PageTitle> | |
| <DiagnosticsView InitialFilter="@Filter" /> | |
| `@page` "/diagnostics" | |
| `@inject` ILogReader Reader | |
| <PageTitle>SharpClient · Diagnostics</PageTitle> | |
| `@if` (Reader.IsAvailable) | |
| { | |
| <DiagnosticsView InitialFilter="`@Filter`" /> | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/SharpClient.UI/Pages/DiagnosticsPage.razor` around lines 1 - 5, Update
the DiagnosticsPage component to inject ILogReader as Reader and conditionally
render DiagnosticsView only when Reader.IsAvailable is true, while preserving
the existing InitialFilter binding.
Comment on lines
+14
to
+15
| public Task<IReadOnlyList<LogEntry>> ReadAsync(int maxEntries = 500) => | ||
| Task.FromResult<IReadOnlyList<LogEntry>>(Entries); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Respect maxEntries in the test reader.
Lines 14-15 return all entries regardless of maxEntries. This prevents tests from modeling a capped reader response and from validating truncation behavior. Return a capped snapshot.
Proposed fix
public Task<IReadOnlyList<LogEntry>> ReadAsync(int maxEntries = 500) =>
- Task.FromResult<IReadOnlyList<LogEntry>>(Entries);
+ Task.FromResult<IReadOnlyList<LogEntry>>(Entries.Take(maxEntries).ToArray());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public Task<IReadOnlyList<LogEntry>> ReadAsync(int maxEntries = 500) => | |
| Task.FromResult<IReadOnlyList<LogEntry>>(Entries); | |
| public Task<IReadOnlyList<LogEntry>> ReadAsync(int maxEntries = 500) => | |
| Task.FromResult<IReadOnlyList<LogEntry>>(Entries.Take(maxEntries).ToArray()); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/SharpClient.UI.Tests/UiFakeLogReader.cs` around lines 14 - 15, Update
UiFakeLogReader.ReadAsync to return a snapshot of Entries limited to maxEntries
rather than the entire collection. Preserve the IReadOnlyList<LogEntry> result
and ensure the original Entries collection is not modified.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds two features: a dedicated screen for composing MUSH poses, and a way to read the on-device diagnostics log after the client has restarted.
Design:
docs/superpowers/specs/2026-08-10-compose-and-diagnostics-design.mdPlan:
docs/superpowers/plans/2026-08-10-compose-and-diagnostics.mdCompose tab
A pose is multi-line prose, but the wire protocol wants one line. Doing that conversion by hand is error-prone, so poses get written elsewhere and pasted in mangled.
/composeis a fifth nav tab whose editor fills the tab body: a prefix chip row on top (say,pose,semipose,@emit, plus a free-text custom prefix remembered per world), aflex: 1middle region holding either the editor or a read-only preview of the exact wire text at the same size, and a footer with a character count,Clear,Preview/Edit, andSend. Ctrl/Cmd+Enter sends; a bare Enter inserts a newline.MushPoseFormatteris a pure static class holding the whole escaping contract: normalise line endings, escape%→%%, trim trailing whitespace and drop leading/trailing blank lines, then replace newlines with%r— in that order, so the inserted markers are not themselves escaped. A prefix ending in=,/, or whitespace joins verbatim (page Bob=→page Bob=He grins…); anything else gets one space.Drafts are held per session, so switching tabs mid-pose doesn't clobber one, and text typed before any session exists is carried into the first session that connects.
Accepted trade-off: literal
%escaping means a deliberately typed%roransi(...)is neutered to%%r. The preview shows the exact wire text, so it is visible before sending rather than surprising afterwards. There is no raw mode.Post-crash diagnostics
The write side already existed — a rotated log under the app's private data dir, fed by
FileLoggerProviderand by theAppDomain/TaskScheduler/AndroidEnvironmentcrash hooks. The only way to read it was the share sheet, so a crash could not be investigated on the device where it happened.FileLogStoreandFileLoggerProvidermove fromSharpClient.AppintoSharpClient.Core, taking the log directory as a constructor argument. Both previously lived in the MAUI head, which no test project can reference — this is what puts the write format and the rotation logic under test for the first time.LogEntryParserreverses the writer's line format, keeping multi-line stack traces attached to their entry, discarding a rotation-severed leading fragment, and skipping a corrupt-but-header-shaped timestamp rather than throwing.ILogReader/FileLogReadermerge the rotated backup and the current file newest-first.WriteExceptionalso drops alast-crash.txtsidecar, so the launch-time check is one small file read and survives rotation./diagnosticslists entries with All / Errors / Crashes filters and Refresh / Copy / Share / Clear. Clear is a two-step button rather than a JSconfirm(), which would block the Blazor circuit inside the Android WebView.CrashBannerrenders inMainLayoutwhen a crash marker is pending and deep-links to/diagnostics?filter=crashes.The Web host has no persistent log and takes
NoopLogReader, so neither the viewer nor the banner appears there.Out of scope: OOM kills and native crashes never reach the managed hooks, so a run ending that way leaves no marker and shows no banner. Detecting that would need a clean-shutdown sentinel.
Incidental fix
3089f49pins AngleSharp 1.5.2 in the bUnit test project. A NuGet advisory (GHSA-pgww-w46g-26qg) against the 1.4.0 that bunit 2.7.2 resolves transitively began failing the audit, andTreatWarningsAsErrorsturns NU1902 into a build error — soSharpClient.UI.Testshad stopped building on every branch,masterincluded. 1.5.0 is the first patched release. Worth dropping the pin when bunit is next bumped to 2.8.6+, which brings a patched AngleSharp transitively.Testing
262 core / 75 UI / 17 data, all green in Release. Web host and Android head both build Release with 0 warnings.
Coverage added: the formatter's escaping order, line-ending forms, and every separator branch;
ComposeViewModelsend-gating, per-session drafts, per-world prefix persistence;FileLogStorewrite and rotation against a temp directory; parser edge cases including multi-line exceptions and corrupt input; reader ordering across the rotation boundary and the crash-marker lifecycle; and bUnit coverage for the compose chord, the diagnostics filters, Share/Copy failure paths, and the banner.Every task was reviewed independently and the whole branch got a final review pass; the fixes from both are in the history.
Still needs a device run. The MAUI-only paths — the real log file, the share sheet, and the banner after an actual crash — are covered by tests and the Android build but have not been exercised on hardware.
Known follow-ups (non-blocking)
DiagnosticsView.Visiblere-filters and re-allocates on every access.FileLogReader.ReadAsyncis synchronous behind an async signature; the read plus parse happens on the UI thread._truncatedis a false positive when the log is exactlyMaxEntrieslong.ReadAsyncthrows on a negativemaxEntries(unreachable today).LogEntryParserdrops blank lines inside a detail block.ComposeViewModel.CustomPrefixpersists on every keystroke.🤖 Generated with Claude Code
https://claude.ai/code/session_015MPRzndB5egy4F2A3q5852
Summary by CodeRabbit